-
Notifications
You must be signed in to change notification settings - Fork 305
Add missing polls LLC operations #5966
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
# Conflicts: # stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/MoshiChatApi.kt # stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/mapping/DomainMapping.kt # stream-chat-android-client/src/test/java/io/getstream/chat/android/client/Mother.kt
|
DB Entities have been updated. Do we need to upgrade DB Version? |
SDK Size Comparison 📏
|
# Conflicts: # stream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.kt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements missing polls LLC (Low Level Client) operations to provide comprehensive poll and poll option management capabilities. The PR adds several new endpoints while maintaining backward compatibility through deprecation warnings.
Key changes include:
- Addition of poll CRUD operations including get, partial update, and query endpoints
- Implementation of poll option lifecycle management (create, update, delete)
- Enhanced poll creation with support for extra data on both polls and options
- Proper offline storage updates and network/domain model extensions
Reviewed Changes
Copilot reviewed 49 out of 49 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| stream-chat-android-ui-components/.../CreatePollViewModel.kt | Updates poll creation to use PollOption objects instead of plain strings |
| stream-chat-android-ui-common/.../MessageListController.kt | Replaces deprecated suggestPollOption with createPollOption |
| stream-chat-android-previewdata/.../PreviewPollData.kt | Adds missing poll properties for preview data consistency |
| stream-chat-android-offline/.../PollsMapperTest.kt | New comprehensive test suite for poll entity mapping |
| stream-chat-android-offline/.../PollsMapper.kt | New mapper for poll entity conversions with proper domain mapping |
| stream-chat-android-offline/.../Poll.kt | Enhanced poll entity with additional fields for complete data storage |
| stream-chat-android-offline/.../MessageMapper.kt | Removes duplicate poll mapping code in favor of dedicated mapper |
| stream-chat-android-offline/.../ChatDatabase.kt | Database schema version bump to accommodate new poll fields |
| stream-chat-android-core/.../Mother.kt | Updates test data factory to include new poll properties |
| stream-chat-android-core/.../Poll.kt | Enhanced poll models with extra data support and comparable field providers |
| stream-chat-android-client/.../ChatClient.kt | Adds comprehensive poll management API methods with proper deprecation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
...hat-android-client/src/main/java/io/getstream/chat/android/client/api/models/PollRequests.kt
Outdated
Show resolved
Hide resolved
…roid/client/api/models/PollRequests.kt Co-authored-by: Copilot <[email protected]>
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md
|
| } | ||
|
|
||
| /** | ||
| * Transforms [DownstreamPushPreferenceDto] to [PushPreference]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * Transforms [DownstreamPushPreferenceDto] to [PushPreference]. | |
| * Transforms [QueryPollVotesResponse] to [QueryPollVotesResult]. |
| pollId: String, | ||
| vote: Vote, | ||
| ): Call<Vote> { | ||
| return api.removePollVote(messageId, pollId, vote.id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about creating:
removePollVote(
messageId: String,
pollId: String,
voteId: String,
):and deprecate:
removePollVote(
messageId: String,
pollId: String,
vote: Vote,
):?
So that would be align with the specs https://www.notion.so/stream-wiki/Polls-c53f9695fb4e4d9083e5b6b2c49017fd#79a88ba6da7e4fbe8b7e05ce119ba1bc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I can do that!
| * @return Executable async [Call] responsible for updating a poll. | ||
| */ | ||
| @CheckResult | ||
| public fun partialUpdatePoll( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will add it!
| @CheckResult | ||
| public fun createPollOption( | ||
| pollId: String, | ||
| option: CreatePollOptionRequest, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is a public API, what do you think about removing the Request from the data class name?
Maybe also consider reusing the PollOption model.
The id could be introduced to PollOption as optional, so it could be reused by create and update, instead of using CreatePollOptionRequest and UpdatePollOptionRequest models
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was actually considering to use the PollOption, but I wasn't; sure about defining it as:
data class PollOption(val id: String? = null)But I was worried about being a bit confusing for usars. Let me try again to see how it would affect the API, and I will get back to you.



🎯 Goal
Implements poll and poll options management operations on the LLC.
Added:
ChatClient.getPoll(pollId: String)ChatClient.partialUpdatePoll(pollId: String, set: Map<String, Any>, unset: List<String>)ChatClient.createPollOption(pollId: String, option: CreatePollOptionRequest)ChatClient.updatePollOption(pollId: String, option: UpdatePollOptionRequest)ChatClient.deletePollOption(pollId: String, optionId: String)ChatClient.queryPollVotes(pollId: String, filter: FilterObject?, limit: Int?, next: String?, sort: QuerySorter<Vote>?)ChatClient.queryPolls(filter: FilterObject?, limit: Int?, next: String?, sort: QuerySorter<Poll>?)PollConfig(used inChatClient.sendPoll) constructor which accepts extra data for the poll and the poll optionsDeprecated:
PollConfigconstructor without extra dataChatClient.suggestPollOptionin favour ofChatClient.createPollOptionResolves: https://linear.app/stream/issue/AND-793/align-llc-polls-api
Docs PR: https://github.com/GetStream/docs-content/pull/690
🛠 Implementation details
Implements the missing poll-related operations, alongside the Retrofit APIs, network/domain models, and offline storage updates. Replaces the usage of the deprecated methods with the new ones.
Resolves:
https://linear.app/stream/issue/AND-793/align-llc-polls-api
🎨 UI Changes
NA
🧪 Testing
Standard poll testing:
Ensure everything works fine
For the new endpoints - they are not linked to the UI, so you need to manually call them and verify they return the expected results.